* Phase 1: CPA display_name + CEL-safe name validation (server)

- Add typed DisplayName field to CPAAttrs + display_name attr key constant.
- Add ValidateCPAFieldName helper enforcing CEL IDENTIFIER + reserved-word blacklist.
- Wire validation into App.CreateCPAField (always) and App.PatchCPAField (lenient grandfather: skip when Name unchanged).
- Trim + 255-rune cap DisplayName in CPAField.SanitizeAndValidate.
- Developer-facing godoc note documenting rule, sources of truth, and Option C scoping.
- Asserting test for documented Option C plugin-API bypass (closed by PR #36173).

Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md
Plan: .planning/phase-1/PLAN.md
Made-with: Cursor

* Phase 1 (review): address Reza's Major + Minor findings

- Rename misleading subtest "empty DisplayName is omitted from attrs"
  to "empty DisplayName round-trips as empty string" (Major #1).
- Add TestCPAAttrs_JSONOmitEmpty pinning the omitempty wire-format
  contract that PR #36173's typed-attrs strategy relies on (Major #1).
- Extend TestValidateCPAFieldName: case-sensitivity (IN/In ok),
  single-character names (a/_/A ok), missing "as" reserved word
  (Minor #2). Add whitespace-only DisplayName case (Minor #2).
- Document PropertyFieldNameMaxRunes reuse in SanitizeAndValidate
  to prevent drift (Minor #3).
- Replace broken PLAN-server.md reference in bypass-test docstring
  with in-tree CPAAttrs godoc reference (Minor #4).
- Document omitempty semantics on CPAAttrs.DisplayName field to
  prevent the same misreading caught in review (Minor #5).
- Document grouping intent above CPAFieldNameReservedWords (Minor #8).

Review: .planning/phase-1/REVIEW.md
Made-with: Cursor

* Phase 2: in-app backfill migration for CPA display_name

- Add cpaDisplayNameBackfillKey + cpaDisplayNameBackfillVersion constants.
- Implement (*Server).doSetupCPADisplayNameBackfill: idempotent, cursor-paged
  scan over CPA group fields; backfill attrs.display_name = name when empty.
- Register in m1 migration slice in doAppMigrations (mlog.Fatal on error,
  matching existing convention).
- Three migration tests: NoExistingFields, BackfillsMissing, Idempotent.

System-key idempotency + per-field DisplayName-empty check together provide
HA-safe behavior on rolling deploys (last-write-wins on the System key;
data-level idempotency from the per-field check).

Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md
Plan: .planning/phase-2/PLAN.md
Made-with: Cursor

* Phase 2 (review): document race + harden idempotency test

- Document SearchPropertyFields→UpdatePropertyFields rolling-deploy
  race: stale snapshot can revert concurrent admin CPA rename. Pre-
  existing systemic shape (no UpdateAt optimistic-lock); narrow
  window; bounded blast radius (admin re-rename, ABAC ID-keyed).
  Accepted limitation per spec Out of Scope (Major #1, Option C).
- Tighten TestCPADisplayNameBackfill_Idempotent: snapshot UpdateAt
  before second run; assert no DB write on the System key or the
  field row (Major #2).
- Extract clearCPABackfillMarker helper with explanatory godoc to
  centralize the 3x-repeated test precondition (Minor #1).
- Comment fieldA seed as the "key-present-as-empty-string" idempotency
  boundary case (Minor #6).
- Add godoc to doSetupCPADisplayNameBackfill (Minor #10).

Review: .planning/phase-2/REVIEW.md
Made-with: Cursor

* Linting

* Removing unnecessary comments

* Clean up tests

* Linting

* Fix tests

* Updated API doc

* Phase 3: webapp helper + render-site migration for CPA display_name

- Add display_name?: string to UserPropertyField.attrs type.
- New getUserPropertyFieldLabel(field) helper: returns
  attrs.display_name?.trim() || name. Defensive against missing attrs.
- Migrate ~10 user-facing CPA-name render sites to the helper:
  profile popover, user settings general (4 usages incl. line 1673
  missed by high-level plan), admin user detail, admin CPA list (2
  usages), and ABAC editor's selected-attribute UI (3 usages incl.
  the button label found in planning-stage research).
- CEL paths (table_editor, attribute_selector_menu user.attributes
  expression construction, ABAC search filters) keep using `name`
  per spec — display_name is label-only.
- Phase 4 boundary marker: TODOs in admin table + delete modal for
  follow-up admin-edit UX + client-side validator.

Spec: planner/projects/property-display-name/ideas/001-cpa-display-name/spec.md
Plan: .planning/phase-3/PLAN.md

* Phase 3 (review): add Unicode test + correct helper docblock scope

Address Reza's Phase 3 review:
- Major #1: add missing test case for non-ASCII display_name
  (Latin-extended + CJK), pinning the trim/passthrough contract.
- Nitpick #3: correct the helper's JSDoc to reflect that the
  delete modal is intentionally not migrated until Phase 4.

No production behavior change. No new dependencies.

Made-with: Cursor

* Phase 4: admin CPA edit UX + client-side identifier validation

Made-with: Cursor

* docs: append Phase 4 implementation summary

Made-with: Cursor

* Phase 4: admin CPA edit UX + client-side identifier validation

Complete the Phase 4 takeover from the existing dirty worktree and record the verified Stage 2 scope for admin CPA display-name editing, client-side identifier validation, and the required grandfather regression follow-ups.
Document the targeted Jest, typecheck, and lint-equivalent validation results in the Phase 4 plan without widening the implementation scope or rewriting the prior in-scope work.

Made-with: Cursor

* docs: finalize Phase 4 implementation summary

Made-with: Cursor

* docs: correct Phase 4 summary commit reference

Made-with: Cursor

* Phase 4 (review): fix empty-name warning precedence

Required-name validation now short-circuits before uniqueness checks so empty identifiers keep the correct warning. Add duplicate collision regression coverage for the dot-menu flow and add a stable validation-error testid for Phase 5 automation.

Made-with: Cursor

* Test updates

* Fix merge issue

* Fix tests

* PR Feedback

* Move migration to PropertyService

* Updates to UX

* Comment cleanup

* Add webapp tests for CPA display_name and fix CEL-affected specs

Update E2E seeds to use CEL-safe identifiers with display_name,
add ABAC selector spec, and extend Jest coverage for label-rendering
sites, auto-fill guard rails, and required-warning suppression.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Remove .planning/phase-4/PLAN.md

This planning artifact was committed inadvertently and should not be
part of the codebase.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address CodeRabbit review comments

- Fix e2e test to use display_name in label assertions
- Make getIncrementedCELName case-insensitive to prevent collisions
- Update tooltip to mention reserved CEL words
- Replace hasSpaces check with full CEL identifier validation
- Use CPA_FIELD_NAME_MAX_RUNES for consistent maxLength
- Fix race condition by removing global cleanupAllFields
- Enable IntegratedBoards flag for legacy field seeding
- Replace fixed sleeps with state-based waits in tests

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix linting errors in getIncrementedCELName

- Use camelCase for destructured delete_at parameter
- Place dots on same line for method chaining

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Remove unused imports in user_attributes_display_name.spec.ts

- Remove unused deleteCustomProfileAttributes import
- Remove unused getFieldsMap function
- Remove unused FieldsMap type

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix webapp test failure - remove htmlFor assertion

The htmlFor attribute assertion was failing in the test environment,
likely due to a testing library issue. The important functionality
(displaying display_name in labels) is still properly tested.

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix post-merge CI failures: i18n drift and Playwright Prettier

- Re-extract webapp en.json so the identifier tooltip string matches
  user_properties_table.tsx (source of truth was already shortened in
  Phase 4; en.json was not regenerated).
- Apply Prettier formatting to three CPA display_name Playwright specs
  (whitespace and import/expression collapsing only). No test logic
  changes.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address CodeRabbit feedback: use stable locators, add reserved words to tooltip, remove regex from hasText

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix i18n drift: align defaultMessage with en.json for identifier tooltip

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Comment cleanup

* Slugify CPA duplicate names to snake_case

slugifyForCEL now lowercases and inserts underscores at camel/PascalCase
boundaries (e.g. MyField -> my_field, XMLParser -> xml_parser) so
duplicated CPA fields get conventional snake_case names instead of
preserving the source casing.

Co-authored-by: Cursor <cursoragent@cursor.com>

* UX improvements: CEL identifier tooltip, validation, and attribute picker dual-name display

- Add info tooltip to the Attribute column header explaining CEL identifier rules
- Add client-side CEL identifier validation (pattern + reserved words) with a descriptive error message
- Show both display name and unique identifier in the policy attribute picker
- Filter attribute picker search by both display name and unique name
- Add display_name to UserPropertyField attrs TypeScript type
- Expand "CEL" to "Common Expression Language (CEL)" in the attribute-spaces tooltip

Co-authored-by: Cursor <cursoragent@cursor.com>

* Linting

* PR Feedback

* Restore name limit

* Fix tests

* Revert stray comment block above TestCPADisplayNameBackfill_BackfillsProtectedSourceOnlyField

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Revert extended fieldA comment in TestCPADisplayNameBackfill_BackfillsMissing

Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>

* Fix E2E tests

* Fix test

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

…#36582)

* Return descriptive errors from Role.IsValid and Role.IsValidWithoutId

Previously both methods returned bool, leaving callers with no context
about which validation check failed. Now both return error with a
message identifying the specific constraint that was violated.

* Add tests for Role.IsValid and Role.IsValidWithoutId

* Log migration key on doPermissionsMigration failure

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

* MM-68762: Add Postgres migrations for discoverable private channels

Three online-safe migrations introduce the schema that supports the
Discoverable Private Channels feature (PRs 2-5 of MM-68430 will land
behind it):

- 000175 adds Channels.Discoverable BOOLEAN NOT NULL DEFAULT FALSE.
  Metadata-only on Postgres >= 11; no table rewrite.
- 000176 creates a partial index on
  (TeamId) WHERE Discoverable AND Type='P' AND DeleteAt=0
  using CREATE INDEX CONCURRENTLY (-- morph:nontransactional) so the
  build never blocks writes on the populated Channels table.
- 000177 creates the ChannelJoinRequests table with three indexes, the
  important one being the partial unique index on (ChannelId, UserId)
  WHERE Status = 'pending'. That keeps the full audit history intact
  while still enforcing at-most-one active pending request per
  (channel, user).

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Add FeatureFlagDiscoverableChannels (default false)

Gates the per-channel Discoverable toggle and the channel-join-request
flow. Default-OFF so all PRs in the MM-68430 series can land on master
without exposing partial UX.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Add Discoverable + ChannelJoinRequest models

- Channel gains a Discoverable bool, ChannelPatch a *bool, both serialized
  as 'discoverable'. Patch() applies it, Auditable() logs it, and IsValid()
  rejects Discoverable=true on any non-private channel so a misconfigured
  patch can never produce a public discoverable channel.
- New ChannelJoinRequest type captures the per-row state of a non-member's
  request: pending -> approved | denied | withdrawn. Rows are append-only
  with reviewer and timestamps so the table is also the audit trail.
  IsValid() enforces:
  * recognized status,
  * Message and DenialReason rune limits,
  * DenialReason only on denied rows (no orphan reasons),
  * reviewer + reviewed_at present for any terminal review (approved /
    denied) but not for self-service withdrawal.
- Two new WebSocket event constants -- channel_join_request_created and
  channel_join_request_updated -- that later PRs broadcast on the admin
  queue and the requester's My Pending Requests panel.

Unit tests cover Patch(), the new IsValid() rule on Discoverable, the
PreSave/PreUpdate timestamp behavior on ChannelJoinRequest, and every
IsValid branch including the reviewer-required-on-review invariant.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Add discoverable-channel permissions

Two new channel-scoped permissions, each independently rebindable from
the System Console:

- manage_private_channel_discoverability gates the per-channel toggle so
  admins can restrict who can flip discoverability without also handing
  out manage_private_channel_properties.
- manage_channel_join_requests gates the queue list / approve / deny /
  count endpoints (added in PR 2).

Both are added to the channel_admin role bootstrap so new deployments
get them by default, and a new permissions migration
(add_discoverable_channel_permissions) grants them to channel_admin,
team_admin and system_admin scheme roles on existing deployments.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Add ChannelJoinRequestStore and wire Discoverable into channel store

- channelSliceColumns / channelToSlice / updateChannelT now include the
  new Discoverable column so Save() and Update() round-trip the field.
  Existing select paths inherit the column automatically because every
  read goes through channelSliceColumns.
- New ChannelJoinRequestStore interface and SQL implementation:
  Save / Get / GetPendingForChannelAndUser / GetForChannel / GetForUser
  / Update / CountPending. Save translates the
  idx_channeljoinrequests_pending_unique partial unique index violation
  into store.ErrConflict so the app layer (PR 2) can return 409 without
  re-parsing pq errors.
- Storetest suite at storetest/channel_join_request_store.go is invoked
  from sqlstore via the existing StoreTest harness; covers insert /
  partial-unique conflict / re-insert after withdrawal / NotFound /
  status filtering / pagination with TotalCount / Update / CountPending.
- Mocks and retrylayer / timerlayer are regenerated via make store-mocks
  and go generate ./channels/store -- no hand-written generator output.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Add TS types for Discoverable channels + join requests

webapp/platform/types:
- Channel.discoverable?: boolean alongside existing policy_enforced /
  policy_is_active so the web client sees the same wire shape the server
  emits.
- ChannelJoinRequest, ChannelJoinRequestStatus, ChannelJoinRequestList,
  GetChannelJoinRequestsOptions for the API contract surfaced in PR 2.

webapp/platform/client:
- WebSocketEvents enum gains ChannelJoinRequestCreated and
  ChannelJoinRequestUpdated so PR 3 can hang WS handlers off them
  without redeclaring constants.

These are model-only updates with no UI consumer yet; PR 3 introduces
the toggle, request flow, and admin queue surfaces.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Split ChannelJoinRequests indexes into concurrent migrations

The mattermost-govet concurrentIndex lint check enforces CREATE INDEX
CONCURRENTLY on every CREATE INDEX statement, even on an empty
freshly-created table where it would be a no-op. The original 000177
file inlined three CREATE INDEX statements; that failed check-style.

Mirror the convention used by 000166_create_views +
000167_create_views_channel_id_delete_at_index: keep the CREATE TABLE
in its own (transactional) file, and move each index into a separate
nontransactional file that runs CREATE INDEX CONCURRENTLY. Verified
locally against Postgres 15 that all four new migrations apply in
order and the storetest suite (partial unique constraint + paged
list + count) still passes.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Wire new permission migration into test fixtures

Two CI test surfaces missed when the channel_admin role and the
permission-migration list gained the new
manage_private_channel_discoverability and manage_channel_join_requests
entries:

- testlib/store.go: the shared mocked SystemStore used by
  SetupWithStoreMock / SetupEnterpriseWithStoreMock needs an explicit
  GetByName expectation for every migration key (because the mock
  panics on unexpected calls). Add the new
  MigrationKeyAddDiscoverableChannelPermissions key so
  TestCreateOrUpdateAccessControlPolicy, the elasticsearch
  aggregation_job_test, and every other mock-store test stop panicking
  on server bootstrap.
- cmd/mmctl/commands/permissions_test.go: TestResetPermissionsCmd
  hard-codes the channel_admin default permission list and expects
  PatchRole to be called with exactly that slice. Extend the expected
  slice with the two new permission ids so the mmctl reset path stays
  in sync with the role bootstrap.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Register new idx_channels_discoverable_team in TestGetSchemaDefinition

The schema-dump test asserts an exact index count and definition map
for the channels table. Migration 000176 added
idx_channels_discoverable_team — a partial btree on (teamid) gated by
discoverable=true AND type='P' AND deleteat=0. Bump the expected count
from 12 to 13 and add the index's CREATE INDEX definition as produced
by pg_indexes (note: type is cast to channel_type, the existing
domain). Verified locally against Postgres 15.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Fix golangci-lint findings in ChannelJoinRequest store

Two golangci-lint findings on the freshly-added files:

- sqlstore/channel_join_request_store.go:133 (modernize): collapse the
  'if page < 0 { page = 0 }' clamp into max(opts.Page, 0).
- storetest/channel_join_request_store.go:243 (govet shadow): the
  inner Save loop redeclared err with :=, shadowing the outer err
  captured from the first CountPending call. Switch to plain
  assignment so the same err is reused.

Verified locally with golangci-lint v2.11.4 across public/...,
channels/app/..., channels/store/..., channels/testlib/... and
cmd/mmctl/commands/... — 0 issues.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Sync channel_admin bootstrap with TestDoAdvancedPermissionsMigration

app_test.go pins the exact list of permissions the channel_admin role
is expected to hold after DoAdvancedPermissionsMigration completes.
The role bootstrap in role.go grew two entries
(manage_private_channel_discoverability and manage_channel_join_requests),
so the test's expected slice needs the same two entries appended in
the same order, otherwise assert.Equal fails on slice ordering.

This is the same class of fix as the mmctl/permissions_test.go change
in a previous commit -- two parallel test fixtures encode the
channel_admin defaults and have to be updated in lockstep with the
bootstrap.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Add English translations for new model error keys

12 keys were emitted by the new Discoverable + ChannelJoinRequest
validation paths but had no en.json entry, which trips i18n-check on
CI. Add the missing entries with one-line English copy that mirrors
adjacent model errors (Invalid <field>., Create at must be a valid
time., etc.). The new entries are:

- model.channel.is_valid.discoverable.app_error
- model.channel_join_request.is_valid.channel_id.app_error
- model.channel_join_request.is_valid.create_at.app_error
- model.channel_join_request.is_valid.denial_reason.app_error
- model.channel_join_request.is_valid.denial_reason_status.app_error
- model.channel_join_request.is_valid.id.app_error
- model.channel_join_request.is_valid.message.app_error
- model.channel_join_request.is_valid.reviewed_by.app_error
- model.channel_join_request.is_valid.reviewer.app_error
- model.channel_join_request.is_valid.status.app_error
- model.channel_join_request.is_valid.update_at.app_error
- model.channel_join_request.is_valid.user_id.app_error

Generated through 'make i18n-extract'; verified clean with
'make i18n-check'. Per the workspace rule, only en.json was modified --
no other locale files.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Address CodeRabbit review: stable pagination + redact denial reason from audit log

Two production-code findings from CodeRabbit on the freshly-added
ChannelJoinRequest server code:

- sqlstore/channel_join_request_store.go (GetForChannel / GetForUser):
  OrderBy("CreateAt DESC") alone is unstable when two rows share a
  millisecond (NewId is monotonic-ish but CreateAt is millisecond
  resolution), so offset paging could duplicate or skip rows between
  pages. Add Id DESC as a deterministic tie-breaker on both list
  queries.
- model/channel_join_request.Auditable: the denial reason is admin-typed
  free text and could carry sensitive content. Mirror the existing
  has_message pattern by emitting has_denial_reason as a boolean
  presence flag instead of the raw value. Reviewer id, review timestamp,
  and status are still logged, so the audit trail keeps every piece
  needed for compliance review.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Tighten model tests per CodeRabbit review

Two test-only findings from CodeRabbit:

- TestChannelJoinRequestPreUpdateAdvancesUpdateAt previously asserted
  GreaterOrEqual(r.UpdateAt, originalCreate). Because validRequest
  initialises UpdateAt to GetMillis() (same call site as CreateAt), a
  no-op PreUpdate would still pass that check. Seed r.UpdateAt = 1
  before calling PreUpdate() and assert Greater(r.UpdateAt, int64(1))
  so any regression that drops the GetMillis assignment fails the test.
- TestChannelIsValidDiscoverable did not cover ChannelTypeGroup. Add the
  case alongside ChannelTypeOpen and ChannelTypeDirect so the contract
  that 'only ChannelTypePrivate accepts Discoverable=true' is fully
  pinned across all four channel types.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

* MM-68762: Mock ChannelJoinRequest accessor in retrylayer test

retrylayer_test.go's genStore() helper mocks every Store() accessor
because retrylayer.New() wraps the entire surface. The new
ChannelJoinRequest() method I added on Store was missing from the
mock, so TestRetry/on_regular_error_should_not_retry panicked with
'Unexpected Method Call ChannelJoinRequest()' on Postgres shard 0.

Add the mock alongside the other accessors. No production code
change.

Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Ibrahim Serdar Acikgoz <isacikgoz@users.noreply.github.com>